Skip to content

fix(compilers/openapi): refuse a scheme that names no mechanism - #296

Open
OmarAlJarrah wants to merge 1 commit into
mainfrom
fix/openapi-typeless-security-scheme
Open

fix(compilers/openapi): refuse a scheme that names no mechanism#296
OmarAlJarrah wants to merge 1 commit into
mainfrom
fix/openapi-typeless-security-scheme

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Closes #294.

Summary

A components.securitySchemes entry with no type fell into fillSchemeKind's default
branch — the one written for a type the compiler does not recognise, which degrades to
AuthKindCustom carrying the raw token. With no type at all there is no token, so the
entry interned as a custom scheme whose mechanism was the empty string, and a requirement
naming it got a live AuthID. The IR stated that the API was authenticated by a scheme
that named nothing.

type: http with no scheme reaches the same state through fillHTTPScheme's default
branch. It is the same defect at a second site and is fixed with it.

Both are now refused. Nothing is interned, the entry is reported at its own components
pointer under a new openapi/incomplete-security-scheme code, and a requirement naming it
drops whole exactly as one naming an undeclared scheme already does.

Why refuse rather than intern with a distinguishable spelling. AuthKind has no value
meaning "the document did not say" — every kind names a mechanism, and AuthKindCustom
names one the IR does not model rather than one the entry never gave. Interning it would
put a scheme no emitter can implement in Document.Auth, recognisable only by an empty
Scheme field: an in-band error for every consumer to rediscover. Adding an enum member
for it would put a description of a broken document in the channel that describes APIs,
when the diagnostics channel already exists for that.

This reuses the remedy an option naming an undeclared scheme already gets, and does so
deliberately rather than by inheritance. The two faults differ — that entry was never
declared, this one was declared and left unsaid — but the IR has no more room for the
second than for the first, and the cost is one already documented and diagnosed at every
step: a requirement naming the entry drops whole, and a list whose every option drops
collapses to nil rather than to the empty-option encoding that reads as "no auth is also
fine". The reasoning is recorded at mechanismRefusalDiag.

The declared fields no longer vanish

The same branches dropped whatever else the entry wrote, and so did every other branch.
Each mechanism's lowering reads only the fields its own type defines — in/name under
apiKey, flows under oauth2, and so on — so a document writing them anywhere else
lost them outright: no IR field, no Unmodeled entry, no diagnostic. That is a silent drop
of declared source text.

Every mechanism field a type does not define is now kept verbatim under Unmodeled at
openapi:<field>, located at the field's own pointer, marked ReasonDegradedLowering, and
announced with an info diagnostic. ir.AuthScheme is flat and does hold a field of each
name, but filling one would say the mechanism has a property it does not define — an apiKey
location on a scheme that is not an apiKey — so the declaration is kept beside the scheme
rather than inside it.

The one collision is deliberate: an unrecognised type already spends AuthScheme.Scheme on
the type token, so a scheme written beside it is kept raw even though the same field
would have held it under type: http. Which one wins is unchanged.

The per-type field list is held to the upstream SecurityScheme model by reflection, so a
field that model gains fails a test rather than silently vanishing — the shape of this bug,
reintroduced by an upstream bump.

Deliberately out of scope

irverify has no AuthScheme rules at all, so nothing structurally asserts that an
interned scheme names a mechanism. The compiler-side refusal closes the instance; the class
is filed as #295, which also notes why such a rule would not have caught this one through
the harness (every document that reaches this state carries a loader error, and
harness.Check returns at the first one).

Test plan

  • gofmt, go vet, golangci-lint run (0 issues), go build ./..., and
    ./scripts/check-coverage.sh all clean — 100% of 4942 statements.
  • New unit coverage for both refused shapes, for the requirement they drop, for the
    preserved fields across all six type classes (each declaring all seven mechanism fields,
    so every row asserts both halves of the one-home rule), for the extension and preserved
    fields sharing one map, and for a field whose value JSON cannot hold being reported
    rather than announced as kept.
  • The conformance corpus gains a scheme declaring a field its type does not define, which
    is what puts the lossless-capture claim in the corpus; the golden records it.
  • Verified by planting each defect and watching the suite redden: the typeless entry
    interning again, the http entry interning again, the preservation removed, the wrong
    Unmodeled reason, an unrecognised type keeping nothing, the refused entry interned
    anyway, extensions clobbering the preserved fields, apiKey wrongly claiming
    bearerFormat, the corpus dropping the stray field, a mislocated entry, the refusal
    downgraded to a warning, and the announcement removed. All twelve were killed, each by
    the test named for it and by an assertion rather than a compile error. The reflection
    guard was checked in both directions.

A securitySchemes entry with no `type` fell into fillSchemeKind's default
branch, where the unrecognised-type degradation interned it as a custom
scheme whose mechanism was the empty string. A requirement naming it got a
live AuthID, so the IR stated that the API was authenticated by a scheme
that named nothing. `type: http` with no `scheme` token reached the same
state through fillHTTPScheme's default branch.

Both are now refused: nothing is interned, the entry is reported at its own
components pointer under a new openapi/incomplete-security-scheme code, and
a requirement naming it drops whole as one naming an undeclared scheme
already does. AuthKind has no value meaning "the document did not say" —
AuthKindCustom names a mechanism the IR does not model, not one the entry
never gave — and inventing one would put a scheme no emitter can implement
in Document.Auth, recognisable only by an empty Scheme field.

The same branches also dropped whatever else the entry declared. Each
mechanism's lowering reads only the fields its own type defines — `in` and
`name` under apiKey, `flows` under oauth2 — so a document writing them
elsewhere lost them with no field, no Unmodeled entry and no diagnostic.
Every field a type does not define is now kept verbatim under Unmodeled
with ReasonDegradedLowering, located at the field itself and announced.
Filling the IR field of the same name instead would say the mechanism has a
property it does not define.

The list of per-type fields is held to the upstream model by reflection, so
a field that model gains fails a test rather than silently vanishing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

openapi: a securityScheme with no type interns as a custom scheme naming nothing

1 participant